feat(): Add trailing newline when saving package.json #4072
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
KristianDD
reviewed
Oct 29, 2018
lib/common/file-system.ts
Outdated
| @@ -205,7 +207,13 @@ export class FileSystem implements IFileSystem { | |||
| space = this.getIndentationCharacter(filename); | |||
| } | |||
|
|
|||
| return this.writeFile(filename, JSON.stringify(data, null, space), encoding); | |||
| let stringifiedData = JSON.stringify(data, null, space); | |||
There was a problem hiding this comment.
Maybe this should be moved inside an else statement. No need to stringify data multiple times.
let stringifiedData;
if (path.basename(filename) === "package.json") {
const newline = detectNewline(stringifiedData);
stringifiedData = stringifyPackage(data, space, newline);
} else {
stringifiedData = JSON.stringify(data, null, space);
}
KristianDD
reviewed
Oct 29, 2018
lib/common/file-system.ts
Outdated
| @@ -205,7 +207,13 @@ export class FileSystem implements IFileSystem { | |||
| space = this.getIndentationCharacter(filename); | |||
| } | |||
|
|
|||
| return this.writeFile(filename, JSON.stringify(data, null, space), encoding); | |||
| let stringifiedData = JSON.stringify(data, null, space); | |||
| if (path.basename(filename) === "package.json") { | |||
There was a problem hiding this comment.
package.json might be used from constants:
import { PACKAGE_JSON_FILE_NAME } from "../constants";
if (path.basename(filename) === PACKAGE_JSON_FILE_NAME ) {
...
}
KristianDD
reviewed
Oct 29, 2018
package.json
Outdated
| @@ -37,6 +37,7 @@ | |||
| "cli-table": "https://github.com/telerik/cli-table/tarball/v0.3.1.2", | |||
| "color": "3.0.0", | |||
| "colors": "1.1.2", | |||
| "detect-newline": "^2.1.0", | |||
There was a problem hiding this comment.
It is safer to use an exact version. You can use npm i detect-newline --save-exact. Same goes for stringify-package dependency.
DimitarTachev
approved these changes
Nov 2, 2018
|
Hi @Logikgate , Congratulations on being one of the winners in the {N} First-time contributors contest! You can claim your prize by contacting us at nativescriptwinners[at]progress.com not later than Nov 30th 2018 . Make sure you send us the following info: Best regards, |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
PR Checklist
What is the current behavior?
Nativescript-CLI commands that save package.json are stripping the trailing newline that NPM inserts when saving package.json. This can cause un-needed commits of package.json where the only thing that has changed is the removal of the trailing newline.
What is the new behavior?
This pull request uses the module stringify-package, the same one used by NPM, to stringify package.json so that the formatting is kept the same. The method writeJSON now uses stringify-package only if it is saving a file named package.json.
Fixes/Implements/Closes #4049 .